Repository navigation
test(straiker): assert a saved api_version v1 with an sk_agt_ key routes to v3 - #44153
Conversation
…tes to v3 (#44106) Co-authored-by: yucheng <yucheng@berri.ai> Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
|
|
There was a problem hiding this comment.
🔍 Devin Review: 2 flags
Not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
…after a kill Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2a5ff3d. Configure here.
TLDR
Problem this solves:
sk_agt_keyintegration-extensionsis red on main at 826b21a (35/36)How it solves it:
psutil.ZombieProcesswhile it waits for the respawnUser Flow
Before: a maintainer watching main after the #44011 merge sees the extensions suite red
integration-extensionson maintest_explicit_api_version_v1_overrides_key_prefixfails: the sink saw a v3 call, the test wanted the v1 webhookAfter: the suite describes the shipped behavior and is green
integration-extensionson maintest_saved_api_version_v1_with_v3_key_routes_to_v3_not_the_v1_webhookpasses with the rest of the fileRelevant issues
Follow-up to #44011 and #44106 (merged into the mirror branch, not main)
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Screenshots / Proof of Fix
Test-only change, so the proof is the integration rig from
tests/integration/README.md: a real proxy with two workers, real Postgres and Redis, the scripted upstream and a local Straiker sink, no mocks inside litellmBefore (826b21a, main)
python -m pytest tests/integration/observability/test_straiker_v3_platform.py::test_explicit_api_version_v1_overrides_key_prefix -q1 failed in 25.95s: the saved-v1 guardrail posted to/api/v3/detect, zero v1 webhook calls at the sinkAfter (2a5ff3d)
python -m pytest tests/integration/observability/test_straiker_v3_platform.py -q(run id 949d1e90b65e4beb8698b4a692b1609c, proxy at 193bd1d, two workers)36 passed in 77.50s, then a second run with the same selection36 passed in 73.21s, identical collected and passed sets, no skipspython tests/integration/run.py extensions tests/integration/observability/test_straiker_v3_platform.py tests/integration/observability/test_guardrail_timeout_all_providers.py:81 passed in 97.00sintegration-extensionson this branch at 193bd1d (pipeline 90756): all 81 Straiker and guardrail-timeout cells green excepttest_burst_survives_one_worker_kill, which hitpsutil.ZombieProcesson the killed worker; b229ca0 fixes that cell and 2a5ff3d only reworded the comment above the renamed cell and the file runs36 passed in 70.49sthroughrun.pyagain. CircleCIintegration-extensionsat b229ca0 (pipeline 90762): all 81 Straiker and guardrail-timeout cells green; the only reds in that job are two unrelated observability cells that also fail on mainType
✅ Test
Final Attestation
Link to Devin session: https://app.devin.ai/sessions/16f17e78d5b6462991838dcfb24e3249
Open in Devin Desktop: https://app.devin.ai/desktop/session/16f17e78d5b6462991838dcfb24e3249?variant=devin
Requested by: @yucheng-berri
Note
Low Risk
Test-only changes to integration assertions and worker discovery; no production code paths modified.
Overview
Aligns Straiker integration tests with shipped routing: a guardrail saved with
api_version="v1"but ansk_agt_key must hit v3 detect, not the v1 webhook.The old cell expected explicit v1/litellm webhook behavior; it is replaced by
test_saved_api_version_v1_with_v3_key_routes_to_v3_not_the_v1_webhook, which covers allow (200 + one v3 call, provider reached, zero v1 sink hits) and block (400 + block message, v3 only, no provider).Separately hardens the worker-kill chaos test:
_uvicorn_workersnow uses_is_live_workerto ignore zombie children and toleratepsutilerrors while waiting for respawn, avoiding flakypsutil.ZombieProcessafterSIGKILL.Reviewed by Cursor Bugbot for commit 2a5ff3d. Bugbot is set up for automated code reviews on this repo. Configure here.